Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Dionisia - Edges - Inspiration-Board #30

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open

Dionisia - Edges - Inspiration-Board #30

wants to merge 3 commits into from

Conversation

larachan15
Copy link

Inspiration Board

Congratulations! You're submitting your assignment!

Comprehension Questions

Question Answer
Explain the steps in creating a new Card from the form. When a user fills out the form and hits the submit button, this triggers the onFormSubmit event handler. It creates a newCard object. This is added to the API database by using the updated state as a parameter for the callback function addCard.
How did you learn how to use the API? I used Postman and I also read the API documentation
What function did you use to place the GET request from the API to get the list of cards? Why use that function? I used componentDidMount to get the list of cards from the API. I am not sure I fully understand how this works though.
Explain the purpose of a Snapshot test. It's to help us determine if our HTML we expect to render is rendered correctly. We can use it to compare the previous JSX that was rendered to the current JSX rendered.
What purpose does Enzyme serve in testing a React app? Enzyme helps us create two types of rendering of our components: deep(renders the component and all its children) and shallow(renders the component, but not the children).

@droberts-sea
Copy link

Inspiration Board

What We're Looking For

Feature Feedback
Core Requirements
Git hygiene I would like to see much more frequent commits
Comprehension questions yes
General
Card Component renders the data provided as props yes
Board Component takes a URL and renders the list of Cards and passes in callback functions yes
NewCardform Component is a controlled form and uses a callback function to return entered data to the parent component yes
API
GET request made in componentDidMount yes
DELETE request made in callback function yes
POST request made in callback function passed to NewCardForm component. yes
Snapshot testing yes
Styling yes
Overall Good work overall. I've left a couple of inline comments below, but in general it's clear to me that the learning goals around integrating a React app with an API have been met. Keep up the hard work!

droberts-sea
droberts-sea reviewed Dec 20, 2018
View reviewed changes
src/components/Board.js
axios.delete(DELETE_CARD)
.then(() => {
const cardsArray = this.state.cards
console.log(cardsArray.length);
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I like that you've kept all the API interaction logic in one place - the callbacks are a little more complex, but I would say it makes the app as a whole much easier to comprehend. Whether or not you intended it, this is a great example of the container component pattern well-applied.

src/components/Card.js
@@ -5,17 +5,50 @@ import emoji from 'emoji-dictionary';
import './Card.css';

class Card extends Component {
deleteCardHandler = () => {
console.log("Delete button was clicked on");

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could this be a functional component?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@droberts-sea droberts-sea droberts-sea left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@larachan15 @droberts-sea